fix: persist advance_in_progress to prevent cross-process session skip - #114
Merged
Merged
Conversation
A `vesper ask` CLI process starting a session track cleared Cider's queue, causing the long-lived server's background worker to see is_playing=false during startup and advance prematurely (track skips within ~3s). Two gaps allowed it: - advance_in_progress was in-memory only, never persisted, so the server worker could not see the CLI was mid-advance. - last_advance_at was persisted at the end of track play, so the cooldown check read a stale timestamp during the startup gap. Fix has two layers: 1. Persist advance_in_progress to the session_runtime table (new column + schema migration). Set and persist it True at the start of _play_session_track, clear and persist it False at the end and in the exception handler. _effective_session_runtime reads the persisted value as authoritative so a stale in-memory flag cannot override another process's in-flight advance. 2. Add a SESSION_MIN_PLAY_SECONDS (10s) backstop: don't auto-advance if the current track has played less than the threshold. This catches Cider's noisy playback reporting during buffering/startup independent of the cross-process coordination. Also persist last_advance_at at the start of the advance (not the end) so the cooldown is measured from when the advance began. Tests: +4 covering cross-process advance blocking, min-play-duration guard, storage upsert preservation/clearing, and schema migration backfill. All 285 pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the premature track skip where a session started via
vesper askwould advance to the next track within ~3 seconds.Root cause
A cross-process worker race.
vesper askis a separate CLI process that clears Cider's queue and starts a track. During that ~1-2s startup, Cider reportsis_playing=false. The long-lived server's background worker (separate process) sees the stop and advances prematurely. Two gaps allowed it:advance_in_progresswas in-memory only — never persisted to the DB, so the server worker couldn't see the CLI was mid-advance.last_advance_atwas persisted at the end of track play, so the cooldown check read a stale timestamp during the startup gap.Fix (two layers)
Persist
advance_in_progressto thesession_runtimetable (new column + schema migration). Set/persist itTrueat the start of_play_session_track, clear/persist itFalseat the end and in the exception handler._effective_session_runtimereads the persisted value as authoritative so a stale in-memory flag can't override another process's in-flight advance.Min-play-duration backstop (
SESSION_MIN_PLAY_SECONDS = 10.0): don't auto-advance if the current track has played less than the threshold. This catches Cider's noisy playback reporting during buffering/startup independent of the cross-process coordination.Also persists
last_advance_atat the start of the advance (not the end) so the cooldown is measured from when the advance began.Test commands run
All 285 tests pass (+4 new).
Note
This addresses the premature skip only. The separate "Lizzo cluster" issue (a single descriptive vibe term matching one artist's catalog with no artist diversification) is tracked for a follow-up.